Fix segfault in Tracing JIT with object reference#20948
Closed
khasinski wants to merge 1 commit intophp:masterfrom
Closed
Fix segfault in Tracing JIT with object reference#20948khasinski wants to merge 1 commit intophp:masterfrom
khasinski wants to merge 1 commit intophp:masterfrom
Conversation
When FE_RESET_RW executes, it converts the CV to a reference before checking if the array/object is empty. However, when the JIT creates exit points for FE_RESET_RW in zend_jit_trace_handler(), it wasn't updating the stack type for op1 to reflect this change. This caused side traces compiled from these exit points to have incorrect type information. The side trace's CV cleanup code would see IS_OBJECT and generate a direct call to zend_objects_store_del(), but the actual value was a zend_reference*, causing a segfault. The fix adds ZEND_FE_RESET_RW to the list of opcodes that temporarily set their op1 stack type to IS_UNKNOWN before creating exit points. This follows the same pattern used for ZEND_BIND_INIT_STATIC_OR_JMP. When IS_UNKNOWN, the JIT falls back to SSA type info which correctly includes MAY_BE_REF for FE_RESET_RW's op1_def.
khasinski
force-pushed
the
fix/gh-20818-jit-object-reference
branch
from
e991cfe to
cd2ca57
Compare
dstogov
approved these changes
Jan 19, 2026
Member
dstogov
left a comment
dstogov
left a comment
There was a problem hiding this comment.
The fix is right.
I think it should be merged into PHP-8.4.
Member
|
@khasinski Nice work, thank you very much! |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes #20818
Problem
When
FE_RESET_RW(foreach by-reference) executes, it converts the CV to azend_referencebefore checking if the array/object is empty. However, when the JIT creates exit points forFE_RESET_RWinzend_jit_trace_handler(), it was not updating the stack type for op1 to reflect this change.This caused side traces compiled from these exit points to have incorrect type information. The side trace CV cleanup code would see
IS_OBJECTand generate a direct call tozend_objects_store_del(), but the actual value was azend_reference*, causing a segfault.Solution
Add
ZEND_FE_RESET_RWto the list of opcodes that temporarily set their op1 stack type toIS_UNKNOWNbefore creating exit points. This follows the same pattern already used forZEND_BIND_INIT_STATIC_OR_JMPandZEND_FE_FETCH_R/RW.When
IS_UNKNOWN, the JIT falls back to SSA type info which correctly includesMAY_BE_REFforFE_RESET_RWop1_def (seezend_inference.c:3483-3484).Changes
ext/opcache/jit/zend_jit_ir.c: AddZEND_FE_RESET_RWcase to both switch blocks inzend_jit_trace_handler()that handle stack type updates around exit point creationext/opcache/tests/jit/gh20818.phpt: Test case from the issueTesting
opcache.protect_memory=1